Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved TDoA outlier filter #1237

Merged
merged 9 commits into from
Mar 1, 2023
Merged

Improved TDoA outlier filter #1237

merged 9 commits into from
Mar 1, 2023

Conversation

krichardsson
Copy link
Contributor

This PR contains an updated outlier filter for TDoA samples in the kalman filter. It has generally been simplified and now better supports various packet rates and nose levels in the system. This should make it work with TDoA long range and in systems with many anchors.

Closes #1158

Copy link
Member

@knmcguire knmcguire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple of comments on these!

@@ -71,6 +76,7 @@ void kalmanCoreUpdateWithTDOA(kalmanCoreData_t* this, tdoaMeasurement_t *tdoa)
h[KC_STATE_Y] = (dy1 / d1 - dy0 / d0);
h[KC_STATE_Z] = (dz1 / d1 - dz0 / d0);

#if USE_STEP_OUTLIER_FILTER
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be part of the kbuild config?

@@ -29,6 +29,7 @@

bool outlierFilterValidateTdoaSimple(const tdoaMeasurement_t* tdoa);
bool outlierFilterValidateTdoaSteps(const tdoaMeasurement_t* tdoa, const float error, const vector_t* jacobian, const point_t* estPos);
bool outlierFilterValidateTdoaIntegrator(const tdoaMeasurement_t* tdoa, const float error, const uint32_t nowMs);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a comment, with kalmanCoreUpdateWithTDOA we use all caps for TDOA, but here it is only Tdoa. Shouldn't all be TDOA for concistency?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could make a separate PR for this, it might be too much of a rabbit hole for now.

}


// Integrator TDoA outlier filter -------------------------------------
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great comments! But perhaps it should be more according to doxygen convention and put it closer to the function. If we ever want to make doxygen comments for these, it might get lost



// The maximum size of the integrator. This size determines the time [in ms] needed to open/close the filter.
static const float INTEGRATOR_SIZE = 300.0f;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we should make a struct for these values and initialize the avalues when the kalman filter is initialize? What do you think? It will make it more suitable for wrapping and the bindings?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also it would make it possible to use this filter for something else

@krichardsson
Copy link
Contributor Author

  • Moved the filters into separate files and generally refactored the code.
  • Put the data for the new filter in a struct while leaving the old filter as it is (it will be removed soon anyway).
  • Added configs to kbuild

Copy link
Member

@knmcguire knmcguire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll also give it a quick flight test

* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*
* outlierFilter.c: Outlier rejection filter for the LPS system
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for the lighthouse system

Copy link
Member

@knmcguire knmcguire left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

flight test is okay! If the copy paste error I mentioned before is fixed go ahead and merge it

@krichardsson
Copy link
Contributor Author

Fixed the comment, and some other I found as well

@krichardsson krichardsson merged commit 6490515 into master Mar 1, 2023
@krichardsson krichardsson deleted the krichardsson/issue-1158 branch March 1, 2023 10:16
@knmcguire knmcguire added this to the next-release milestone Apr 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TDoA3 outlier filter need update for Long Range
2 participants